Fix schedule and then publish flow #11013
Conversation
There was a bug that happened when: - a post was scheduled for a future date from now - then the data changed to a past date from now At this point, the post couldn't be published directly by clicking the "Publish" button. This component would show the post-publish panel with a "Post scheduled" status.
oandregal
added
[Type] Bug
Although the post status is 'future', the data is before now, so we should show the pre-publish panel with the Publish button.
| }; | ||
| } | ||
|
|
||
| static getDerivedStateFromProps( props, state ) { |
oandregal
changed the title
There was a problem hiding this comment.
This is looking great! When I test this locally I'm unable to reproduce the bug by following the steps in #10930.
Thanks for adding unit tests and for simplifying PostPublishPanel. It's a lot easier to understand how the component works now that there's no state 😍🌈
I left some minor comments which should be fixed up before merging. Giving you the 👍 though so that you're not blocked on me re-reviewing.
packages/editor/src/components/post-publish-panel/test/index.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/post-publish-panel/test/__snapshots__/index.js.snap
Outdated
Show resolved
Hide resolved
| } = select( 'core/editor' ); | ||
| const { isPublishSidebarEnabled } = select( 'core/editor' ); | ||
| return { | ||
| postType: getCurrentPostType(), |
There was a problem hiding this comment.
Removed, was a leftover.
| isPublished: isCurrentPostPublished(), | ||
| isScheduled: isCurrentPostScheduled(), | ||
| isSaving: isSavingPost(), | ||
| isBeingScheduled: isEditedPostBeingScheduled(), |
There was a problem hiding this comment.
In the component, we were using only isScheduled to determine whether a post is scheduled, but we also need to take into account whether the date scheduled is > now (which is what isBeingScheduled does).
| if ( | ||
| state.submitted || | ||
| props.isSaving || | ||
| ( ! props.isPublished && ! props.isScheduled ) |
There was a problem hiding this comment.
This logic wasn't accounting for the fact that an user can:
- schedule a post with a date in the future
- change its mind and modify the date to be in the past
After the second step, the user should be able to publish directly but wasn't.
mmm, I could repro that the bug exists and that this PR fixes it.
ha, Australia-Spain connection, antipodean collaboration, the best kind of async! ⌚ 🌏 |
Yes, to clarify, I meant that I could not reproduce the bug when running this branch which fixes it 😀 |
* Use props instead of derived state to render the component. There was a bug that happened when: - a post was scheduled for a future date from now - then the data changed to a past date from now At this point, the post couldn't be published directly by clicking the "Publish" button. This component would show the post-publish panel with a "Post scheduled" status. * Test: check that pre-publish panel is shown * Test: check that post-publish panel is shown if post is published * Test: check that post-publish panel is shown if post is scheduled * Test: isScheduled is true, but isBeingScheduled false Although the post status is 'future', the data is before now, so we should show the pre-publish panel with the Publish button. * Fix isSaving logic * Update snapshots * Test: check that spinner is shown if post is being saved * Remove unused prop * Name spinner component so snapshot test is more semantic * Fix typo * Update snapshot
Fixes #10930